Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guidance to use composable functions instead of filter-as-segment feature #520

Open
wants to merge 6 commits into
base: vNext
Choose a base branch
from

Conversation

corranrogue9
Copy link
Contributor

Discussed previously, this PR adds the guidance to avoid the filter-as-segment OData feature and instead use a composable function which accepts an expression parameter for those scenarios where operations on filtered collections are desirable.


Doing this is beneficial due to the robust nature of OData filter expressions: clients will be able to dismiss risky users based on any supported filter without the service team needing to implement a new `dismiss` overload that filters based on the new criteria.
However, there are some concerns about the discoverability of using the filter-as-segment feature, as well as the support of [parameter aliasing](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_ParameterAliases) that's required.
As a result, functions should be introduced that act in the same way as the filter-as-segment:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think naming a specific function as "filter" is very confusing for users who expect a standard $filter behavior. I believe it does not follow our naming guidelines without having a product-specific prefix. Unless we will treat this function similarly to delta?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespace qualification of functions is optional in msgraph.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customFilter or filterFunction

@OlgaPodo OlgaPodo added Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team. labels Feb 1, 2024
@@ -0,0 +1,61 @@
# Filter as segment

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this positioned as an article? i think it make sense to create a pattern and clearly specify when this solution is a good one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be included as another scenario of #512
We might rename 512 slightly to reflect this, but doesn't seem like this should be a separate pattern, when the pattern is "replacing a complex filter with a function".


Doing this is beneficial due to the robust nature of OData filter expressions: clients will be able to dismiss risky users based on any supported filter without the service team needing to implement a new `dismiss` overload that filters based on the new criteria.
However, there are some concerns about the discoverability of using the filter-as-segment feature, as well as the support of [parameter aliasing](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_ParameterAliases) that's required.
As a result, functions should be introduced that act in the same way as the filter-as-segment:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customFilter or filterFunction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants